Skip to content

feat: bootstrap gatekeeper-kit leaf modules - #392

Merged
ndisidore merged 14 commits into
mainfrom
nathan/gatekeeper-kit
Sep 2, 2026
Merged

feat: bootstrap gatekeeper-kit leaf modules#392
ndisidore merged 14 commits into
mainfrom
nathan/gatekeeper-kit

Conversation

@ndisidore

@ndisidore ndisidore commented Aug 31, 2026

Copy link
Copy Markdown
Member

Adds @gadgets/gatekeeper-kit: fourteen standalone leaf modules that factor out the plumbing every gatekeeper re-implements around its service-specific parts. The package is a library and each module takes the narrowest KV surface it needs and works à la carte, with no dependency on the assembly layer that will sit on top.

plans/gatekeeper-kit.md carries the module-by-module rationale, the implementation plan, and future Layer 2 work for the assembly spec itself

Modules

  • Connect flow: nonce minting, constant-time comparison, and the initiation → OAuth handshake as atomic KV transitions
  • Connect pages: the shared close-window, expired-link, and error pages, with escaping and response headers fixed once
  • Credentials: identity-fenced storage, coalesced skew-aware refresh, and the consumer cache that reports a dead grant
  • Credential expiry: arm-fenced notify-once latch that survives eviction and retries after a failed callback
  • Observers: four verification strategies, a batched-access tracker, and the prepare → authorize → commit gate
  • Actions: two-tier journal, declarative apply/reject dispatch, serialized resolution, one post-apply write
  • Simulation: frozen views over pending records, replay that stops at the first unsupported step, provisional IDs that refuse a conflicting rebind
  • Cursors, cache, serial queue, HTTP errors: RpcTarget cursors with serialized paging, generation-keyed TTL cache, the FIFO gate both need, numeric 401/403/404 classification

@github-actions github-actions Bot added delivery Changes to CI or release delivery gatekeeper Changes to a gatekeeper integration labels Aug 31, 2026
@github-actions

Copy link
Copy Markdown

Preview: pr392-nathan-gatekeeper-kit

https://pr392-nathan-gatekeeper-kit-router.cloudflare-os-previews.workers.dev

Dashboard · deleted when this PR closes

@ask-bonk

This comment was marked as outdated.

@ndisidore ndisidore changed the title Nathan/gatekeeper kit feat: bootstrap gatekeeper-kit leaf modules Aug 31, 2026
@ndisidore
ndisidore force-pushed the nathan/gatekeeper-kit branch from 2eaa966 to 2f29a87 Compare August 31, 2026 12:14
@ask-bonk

This comment was marked as outdated.

@ndisidore
ndisidore force-pushed the nathan/gatekeeper-kit branch from 2f29a87 to dbf4007 Compare August 31, 2026 14:49
@ask-bonk

This comment was marked as outdated.

@ndisidore
ndisidore force-pushed the nathan/gatekeeper-kit branch 2 times, most recently from dbf4007 to 4344b75 Compare August 31, 2026 21:21
ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk

This comment was marked as outdated.

@ndisidore
ndisidore force-pushed the nathan/gatekeeper-kit branch from 4344b75 to 28ac1c8 Compare August 31, 2026 22:22
@ask-bonk

This comment was marked as outdated.

@ndisidore
ndisidore force-pushed the nathan/gatekeeper-kit branch from 28ac1c8 to cda0c21 Compare August 31, 2026 23:37
@ask-bonk

This comment was marked as outdated.

@ndisidore
ndisidore force-pushed the nathan/gatekeeper-kit branch from cda0c21 to 2e81580 Compare September 1, 2026 11:48
ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk

ask-bonk Bot commented Sep 1, 2026

Copy link
Copy Markdown

Submitted 3 actionable inline findings. Tests were not run because pnpm is unavailable.

github run

@ndisidore
ndisidore force-pushed the nathan/gatekeeper-kit branch from 2e81580 to 8cf8a2f Compare September 1, 2026 15:28
@ask-bonk

This comment was marked as outdated.

ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk

This comment was marked as outdated.

@ndisidore
ndisidore force-pushed the nathan/gatekeeper-kit branch from 6677e99 to 7b6838a Compare September 1, 2026 16:50
ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk

This comment was marked as outdated.

@ndisidore
ndisidore force-pushed the nathan/gatekeeper-kit branch from 7b6838a to 1ea3f61 Compare September 1, 2026 18:38
ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk

This comment was marked as outdated.

New workspace library @gadgets/gatekeeper-kit holding the provider-independent
pieces every gatekeeper hand-rolls today. This first cut lands the connect-flow
leaves: nonce primitives, the two-stage handshake, the shared connect pages, the
HTTP no-access classifier, the simulation substrate, and the expiry latch.

The latch differs from its prior art: it is set only after the callback RPC
resolves, so a crash mid-notify re-notifies later instead of silencing every
future expiry, and concurrent callers share one in-flight notification.

Tests run as two vitest projects: Node for pure logic, workerd for the modules
that need crypto.subtle.timingSafeEqual.
Four strategies cover the taxonomy every gatekeeper picks from -- private, ACL,
tracked-set, open -- behind one ObserverStrategy seam, plus ObservationGate,
which folds a strategy's exclusions into the approval-queue authorization and
promotes newly-revealed sets only once the overseer has agreed to hide them.

ObserverTracker extends the version it came from in the two ways real
gatekeepers need: the observer and observed-set key families are named
separately (so a port keeps both of its existing prefixes), and the ACL oracle
is batched -- one provider call per admission round rather than per set. A short
or ragged oracle result denies. legacyObservedValue honors a stored `true` as
"observed", and denyMessage names the failing set.
CredentialCoordinator owns the account DO's credential record: skew-aware
refresh, concurrent callers coalesced onto one provider round-trip, and an
identity fence so a reconnect or revoke landing mid-refresh is never clobbered
by the older refresh's result. CredentialsExpiredError is the one signal that
means the grant itself died; anything else propagates with credentials intact.

CredentialSource is the facet/verifier side: a short instance-local cache over
the account DO, and the single place a provider auth failure becomes an expiry
notification.
@ndisidore
ndisidore force-pushed the nathan/gatekeeper-kit branch from 4c46897 to c99fd59 Compare September 2, 2026 18:10
@ask-bonk

ask-bonk Bot commented Sep 2, 2026

Copy link
Copy Markdown
  1. High observer-tracker.ts:231,367: Exact-length array-like verifier results are trusted without Array.isArray(). { length: 1, 0: true } can admit or retain an unauthorized observer.

  2. Medium observers.ts:94, observer-tracker.ts:210,225: Rejected ACL callbacks propagate provider error text, which the Workshop displays verbatim to the collaborator. Convert these to bounded, disclosure-safe errors.

  3. Medium action-journal.ts:216-217: retire() deletes the action before persisting its applied-ID marker. If the final write fails, an applied provider effect loses all journal evidence; retries report an unknown action and rejection may succeed incorrectly.

github run

@ndisidore

Copy link
Copy Markdown
Member Author

Bonk has now reached diminishing returns

High observer-tracker.ts:231,367: Exact-length array-like verifier results are trusted without Array.isArray(). { length: 1, 0: true } can admit or retain an unauthorized observer.

Not a reachable failure mode

Medium observers.ts:94, observer-tracker.ts:210,225: Rejected ACL callbacks propagate provider error text, which the Workshop displays verbatim to the collaborator. Convert these to bounded, disclosure-safe errors.

Pre-existing and every gatekeeper currently does this

Medium action-journal.ts:216-217: retire() deletes the action before persisting its applied-ID marker. If the final write fails, an applied provider effect loses all journal evidence; retries report an unknown action and rejection may succeed incorrectly.

The scenario requires a write split that Durable Object storage makes essentially unreachable. If we cared to fix this, the right place would probably be in the overseer itself.

@ndisidore
ndisidore force-pushed the nathan/gatekeeper-kit branch from c99fd59 to a495023 Compare September 2, 2026 18:36
devin-ai-integration[bot]

This comment was marked as outdated.

@ask-bonk

ask-bonk Bot commented Sep 2, 2026

Copy link
Copy Markdown
  1. Medium packages/gatekeeper-kit/src/action-journal.ts:115: allocate() treats legacy records rejected by upgradeRecord() as absent. With a last-issued counter, it can overwrite that record and reuse its ID, causing an old approval to dispatch a new payload. Check raw key occupancy before staging.

github run

Comment thread packages/gatekeeper-kit/src/connect-nonce.ts
Comment thread packages/gatekeeper-kit/package.json Outdated
ActionJournal owns the queued-action record: sequential ids, the staged ->
pending lifecycle, and listPending() as the input createSimulationView expects.
It is two-tier -- a retained applied record moves out of the pending prefix to a
sibling one, so the pending scan stays bounded by genuinely pending records
however many applied ones accumulate (the shape github already uses). Lookups
check both tiers and never filter by state: the output gate commits the staged
record before the submitAction RPC can leave, so a record still marked staged
may already be pending for the overseer. Retiring the retained tier is consumer
policy -- retention is unbounded and caps are per-vendor.

defineActions turns per-kind apply/reject handlers into the overseer's
callbacks. An apply returns its apply-time artifacts, which the kit persists in
one write together with the state transition, so handlers never touch the
journal mid-apply and there is exactly one writer. An apply that throws leaves
the record for a retry and reports "failed" -- a partial provider effect is when
caches are most stale. afterResolve fires once per resolution because every
gatekeeper invalidates caches after one and the big ones repeat it per branch,
where a forgotten branch is a silent stale read.

Revert is deliberately absent. Reject's variance lives inside a handler body,
which dispatch absorbs; revert's variance lives in record lifecycle, which it
cannot -- five gatekeepers have five incompatible revert/retention behaviours,
so revert belongs at the facet seam as ordinary consumer TypeScript.

Apply is at-least-once: the provider call can succeed and the process crash
before the journal write, and the overseer's retry re-applies. No gatekeeper
solves this today; the kit documents it rather than pretending otherwise.

SerialTaskQueue and displayReason come from the applyActionsThrough work, so the
batch contract can layer onto this later without reshaping the journal.
KvTtlCache holds stable provider metadata in the facet's own storage, keyed
within a generation the caller bumps when an applied action may have invalidated
everything at once -- cheaper and more complete than tracking which entries a
write touched.
StreamingCursor fetches provider pages lazily, overlays simulation onto each
item, and merges simulation-only items at their sort position, so a resource with
a long history returns its first page without reading all of them. Pages emptied
by the filter keep it fetching rather than reporting the end early.

Both extend RpcTarget undecorated; a consumer subclasses and decorates when it
wants validated cursor calls.
Deployable discovery keys on wrangler.jsonc alone (readDeployablePackages),
not on the package name, so gatekeeper-kit is already invisible to the release
pipeline, the dev server and preview configs. AGENTS.md said otherwise --
"Each gatekeeper runs as a separate Cloudflare Worker" -- which misleads a
reader into treating the kit as deployable, or into adding a wrangler.jsonc to
make it "work".

Doing that would make it deployable and, because workerKind classifies by
prefix, a gatekeeper: shortName "kit", a routed BASE_URL, and -- since it is
absent from NO_DEFAULT_CRED_INPUTS -- a wizard demanding CLIENT_ID and
CLIENT_SECRET before anyone could install the instance. CI already caught that,
but only as three "missing fixture bundle" failures that name the symptom
rather than the cause, so pin the invariant where it can say so directly.

The internal repo's gatekeeper-shared is the same shape, so this documents an
existing convention rather than inventing one.
Records the design behind the Layer 1 modules that just landed, in the
docs/ directory alongside the other design docs. The plan drove the
implementation, so committing it puts the reasoning next to the code
rather than leaving it in a scratch file.

Reconciled against the shipped signatures before committing, since a
tracked doc reads as authoritative where an untracked one does not:

* 4.6 credentials: the upgrade() contract returns
  { credentials, legacyKeys } and reads only -- the coordinator performs
  the deletes. Documents the write order in commit()/clear() and why the
  fence goes first.
* 4.7 observers: aclObservers takes hasAccess() (answering, and only a
  literal true admits) rather than a throwing verify(); adds the
  canonicalSetId, maxTrackedSets and concurrency options, and
  observerIds() on the tracker and strategy.
* 4.9 simulation: isProvisional is a constructor option, not a method.
  Corrects the binding key to `${namespace}prov:${id}`.
* 4.10 cache: the API is cached(key, ttlMs, load); there is no public
  get/put pair, and the generation fence is internal rather than the
  caller's obligation.

Also adds a status header (Layer 1 landed, Layer 2 still proposal) and
the 4.8 key-layout tables a port needs: the counter convention is
next-unused in 9 of 12 gatekeepers but last-issued in github, linear and
spotify, where adopting the existing counter key would re-issue the last
ID; and the retained tier's derived prefix matches no gatekeeper's
existing keys.
The §4 sections are meant to track the shipped signatures. The review
pass changed six of them and left two findings deliberately unclosed,
so record both halves.

`submit` is no longer "NOT queued" -- it holds its own serial queue,
separate from the resolution queue. Split the staged-record section
into the half that closes (two submissions racing each other, and why
a live staged record could be pruned) and the half that stays open
(a submission racing its parent's rejection).

Smaller signature corrections: `markFailed` caps the reason it stores;
`retain` no-ops on a failed record; `expiresAt` must be finite;
`SimulationResult`'s incomplete arm carries `partial`, not `value`;
`resolve()` consults the classifier before the binding table; and
`readTextCapped` answers a bodyless response before the Content-Length
check and cancels best-effort.

The two declined findings go in the obligations table with the trigger
that would revive each: abandoned observer admissions holding their
slot, and a dropped action kind stranding its dependents. Both cost a
storage-format change to close, and neither has a consumer today.
@ndisidore
ndisidore force-pushed the nathan/gatekeeper-kit branch from 2bd017f to d184c78 Compare September 2, 2026 20:34
Comment thread packages/gatekeeper-kit/src/credentials.ts
@ask-bonk

ask-bonk Bot commented Sep 2, 2026

Copy link
Copy Markdown

Submitted 1 actionable inline finding.

github run

@ask-bonk

ask-bonk Bot commented Sep 2, 2026

Copy link
Copy Markdown
  1. High actions.ts:26,273: Queues are keyed by ActionJournal object identity. Two wrappers over the same KV journal get independent lanes, allowing duplicate provider applies or pruning in-flight staged records. Key coordination by storage/keyspace or require a singleton journal.

  2. Medium credentials.ts:120,184: Credentials and identity cannot be obtained atomically. A reconnect can interleave after fresh() commits but before the caller reads identity(), pairing principal A’s credentials with principal B’s fence and potentially expiring B after A returns 401.

  3. Medium cursors.ts:48: Provider cursors retain fetchPage closures but expose no disposal hook. Closures commonly own duplicated ApprovalQueue/ObservationGate capabilities, which remain undisposed when the cursor stub is released.

Tests were not run because pnpm is unavailable.

github run

@maxwellpeterson
maxwellpeterson self-requested a review September 2, 2026 21:17
@ndisidore
ndisidore merged commit 0e6aa03 into main Sep 2, 2026
17 checks passed
@ndisidore
ndisidore deleted the nathan/gatekeeper-kit branch September 2, 2026 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

delivery Changes to CI or release delivery gatekeeper Changes to a gatekeeper integration workshop/shared Changes to shared Workshop APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants